-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
fix(compiler-dom): handle v-model + v-bind shorthand type edge case #13170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/runtime-core
@vue/reactivity
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
@@ -63,6 +63,13 @@ describe('compiler: transform v-model', () => { | |||
expect(generate(root).code).toMatchSnapshot() | |||
}) | |||
|
|||
test('input with v-bind shorthand type should use dynamic model', () => { | |||
const root = transformWithModel('<input :type v-model="model" />') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering whether it might be better to write this test with the :type
after the v-model
? Or both?
I understand that the test is following the order from the other tests, but in practice the order :type v-model="model"
already works when using the full compiler (including the vBind
transform), it only fails when the :type
comes after the v-model
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the tests via 78a11fe
""" WalkthroughThe changes update the logic for detecting the Changes
Sequence Diagram(s)sequenceDiagram
participant Template as Template Compiler
participant vModelTransform as v-model Transform
participant findProp as findProp Utility
participant CodeGen as Code Generator
Template->>vModelTransform: Parse <input :type v-model="...">
vModelTransform->>findProp: findProp(node, 'type', false, true)
findProp-->>vModelTransform: Return type prop info (shorthand dynamic)
vModelTransform->>CodeGen: Use V_MODEL_DYNAMIC helper
Assessment against linked issues
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
It seems like we have a lot of these edge cases around Would it be better to handle this at an earlier stage, rather than in the I understand the desire to keep things modular by having |
@skirtles-code |
close #13169
Summary by CodeRabbit
Tests
Bug Fixes